Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add puppet-lint-spaceship_operator_without_tag-check #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bastelfreak
Copy link
Member

I tested this for my own modules already, also heavily used in the
foreman ecosystem.

I tested this for my own modules already, also heavily used in the
foreman ecosystem.
@alexjfisher
Copy link
Member

I do this fairly often though...

@alexjfisher
Copy link
Member

and https://github.com/voxpupuli/puppet-zabbix/blob/372ee213e34d5fb0266c7ee1f8424e2c82d1f1bf/manifests/resources/web.pp#L41

Are there any uses you can find where we should change the code and not just disable the plugin with control comments?

@ekohl
Copy link
Member

ekohl commented Jul 1, 2020

I've recently also been wondering. The alternative often isn't better.

https://github.com/voxpupuli/puppet-grafana/blob/900babdccd788b139e57de4c4d7c337ad6f527a6/manifests/init.pp#L182

I think the alternative here is autorequire in the type. Not sure about autonotify though.

@alexjfisher
Copy link
Member

If the plugin could be configured to just check for gross misuse like File <||> and Package <||>...

@alexjfisher
Copy link
Member

I think the alternative here is autorequire in the type. Not sure about autonotify though.

That only works for native (not defined) types though.

@alexjfisher
Copy link
Member

@alexjfisher
Copy link
Member

I think the alternative here is autorequire in the type. Not sure about autonotify though.

That only works for native (not defined) types though.

and presumably all the negatives the plugin is trying to avoid (realisation of virtual resources), still apply if the type uses autorequire??

@ghoneycutt
Copy link
Member

In my nrpe module i have the plugin define include the main class and require a directory, which requires the nrpe package. Which is a long way of saying, we can and should refactor these. +1 for adding this lint plugin.

Seems that these are all using the non-idempotent terminology and horrible pattern of ::install, ::config, ::service. We shouldn't be using that and by moving away from it, we will not need all this spaceship nonsense.

https://github.com/ghoneycutt/puppet-module-nrpe/blob/master/manifests/plugin.pp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants